Immutable DataArrays for RF data #2715
Conversation
dmarek-flex
changed the title
dmarek-flex
force-pushed
the
dmarek/rf_arrays
branch
from
16bf05f to
0847230
Compare
|
@daquinteroflex @yaugenst-flex Do you think this approach is better for packaging microwave data arrays? If yes, I will continue by improving some of the data arrays used and produced by the |
There was a problem hiding this comment.
Until we reorganise https://github.com/flexcompute/tidy3d/blob/develop/tidy3d/components/data/data_array.py it should go there. Note the other voltage, index spatial data arrays. Can we reuse?
There was a problem hiding this comment.
I think it's fine to start formalizing these classes even if it's more code, as long as we can reuse or interoperate with the existing ones
There was a problem hiding this comment.
There was a problem hiding this comment.
Oh that could get confusing. Looks like those VoltageArrays are arrays that have coordinates of voltage, whereas the VoltageArrays I want to introduce hold values related to voltage (with coordinates of freq/time/mode_index)
There was a problem hiding this comment.
I feel like this is somewhat related to this: https://github.com/flexcompute/tidy3d-core/issues/863
Maybe it is a good time to think about a holistic approach to all this?
| Result of voltage computation over remaining dimensions (frequency, time, mode indices). | ||
| """ | ||
| AxisAlignedPathIntegral._check_monitor_data_supported(em_field=em_field) | ||
| voltage = -1.0 * self.compute_integral(field="E", em_field=em_field) | ||
| voltage = VoltageIntegralAxisAligned._set_data_array_attributes(voltage) | ||
| voltage = VoltageIntegralAxisAligned._make_result_data_array(voltage) |
There was a problem hiding this comment.
Is it fine by you that we make this a standalone function rather than a class instantiaton? Like we do for the TerminalComponentModelerData methods https://github.com/flexcompute/tidy3d/blob/bfe45d89cbfd65ead7699b74441157f28f217dc0/tidy3d/plugins/microwave/path_integrals.py#L208C1-L215C69
There was a problem hiding this comment.
ran into some circular dependency issues, but this is closer to what you want
| def _make_result_data_array(result: DataArray) -> ImpedanceResultTypes: | ||
| """Helper for creating the proper result type.""" | ||
| cls = ImpedanceFreqDataArray | ||
| if "t" in result.coords: | ||
| cls = ImpedanceTimeDataArray | ||
| if "f" in result.coords and "mode_index" in result.coords: | ||
| cls = ImpedanceFreqModeDataArray | ||
| return cls.assign_data_attrs(cls(data=result.data, coords=result.coords)) |
There was a problem hiding this comment.
Can we move this to utils as a standalone function?
| def _make_result_data_array(result: DataArray) -> IntegralResultTypes: | ||
| """Helper for creating the proper result type.""" | ||
| cls = FreqDataArray | ||
| if "t" in result.coords: | ||
| return TimeDataArray(data=result.data, coords=result.coords) | ||
| cls = TimeDataArray | ||
| if "f" in result.coords and "mode_index" in result.coords: | ||
| return FreqModeDataArray(data=result.data, coords=result.coords) | ||
| return FreqDataArray(data=result.data, coords=result.coords) | ||
| cls = FreqModeDataArray | ||
| return cls.assign_data_attrs(cls(data=result.data, coords=result.coords)) |
There was a problem hiding this comment.
Same this should be a standalone function in utils?
daquinteroflex
force-pushed
the
dario/rf_endpoints
branch
from
b4a0c6c to
9b7182d
Compare
fix setting data attributes
dmarek-flex
force-pushed
the
dmarek/rf_arrays
branch
from
0847230 to
a5a99e0
Compare
daquinteroflex
force-pushed
the
dario/rf_endpoints
branch
from
b7e2a38 to
43dcd42
Compare
WIP attempt at providing immutable
DataArrayfor microwave quantities. The goal here is to get rid of methods likeImpedanceCalculator._set_data_array_attributes, which add some simple data attributes for users.This can work, but it just seems like a lot of code to get some fairly simple functionality, namely providing units and name for a
DataArray.